Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PRESS12-44 Added a GA event for WooPay connection. #114

Merged
merged 8 commits into from
Jan 9, 2025
Merged

PRESS12-44 Added a GA event for WooPay connection. #114

merged 8 commits into from
Jan 9, 2025

Conversation

mamatharao05
Copy link
Contributor

@mamatharao05 mamatharao05 commented Nov 26, 2024 •
edited
Loading

Proposed changes

Added a GA event for WooCommerce payment connection.
https://jira.newfold.com/browse/PRESS12-44

Type of Change

Production

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update
  • Refactoring / housekeeping (changes to files not directly related to functionality)

Development

  • Tests
  • Dependency update
  • Environment update / refactoring
  • Documentation Update

Video

Checklist

  • I have read the CONTRIBUTING doc
  • I have viewed my change in a web-browser
  • Linting and tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@BrianHenryIE
Copy link
Contributor

I took a look around and I'm not sure that this will work. Please add instructions / tests showing how it works.

The only place I see that option name is in Automattic/woocommerce-payments ... class-wc-payments-account.php#L38 which is used when calling woocommerce/woocommerce ... core-functions.php#L53 and when I follow that, I don't see update_option() anywhere.

@mamatharao05
Copy link
Contributor Author

I took a look around and I'm not sure that this will work. Please add instructions / tests showing how it works.

The only place I see that option name is in Automattic/woocommerce-payments ... class-wc-payments-account.php#L38 which is used when calling woocommerce/woocommerce ... core-functions.php#L53 and when I follow that, I don't see update_option() anywhere.

Checking

@mamatharao05
Copy link
Contributor Author

mamatharao05 commented Dec 5, 2024 •
edited
Loading

I took a look around and I'm not sure that this will work. Please add instructions / tests showing how it works.

The only place I see that option name is in Automattic/woocommerce-payments ... class-wc-payments-account.php#L38 which is used when calling woocommerce/woocommerce ... core-functions.php#L53 and when I follow that, I don't see update_option() anywhere.

@BrianHenryIE I have updated the hook. wcpay_account_data is the option, which gets updated when WooPay is connected. Could you please review it?

@wpscholar wpscholar removed their request for review December 9, 2024 17:20
@mamatharao05
Copy link
Contributor Author

I took a look around and I'm not sure that this will work. Please add instructions / tests showing how it works.
The only place I see that option name is in Automattic/woocommerce-payments ... class-wc-payments-account.php#L38 which is used when calling woocommerce/woocommerce ... core-functions.php#L53 and when I follow that, I don't see update_option() anywhere.

@BrianHenryIE I have updated the hook. wcpay_account_data is the option, which gets updated when WooPay is connected. Could you please review it?

@BrianHenryIE, @circlecube Could you please review?

@mamatharao05
Copy link
Contributor Author

@BrianHenryIE Could you please review this updated PR?

@wpscholar
Copy link
Member

@mamatharao05 - I've requested a few things in the ticket here: https://jira.newfold.com/browse/PRESS12-44

@mamatharao05
Copy link
Contributor Author

@mamatharao05 - I've requested a few things in the ticket here: https://jira.newfold.com/browse/PRESS12-44

Addressed the suggestion.

wpscholar
wpscholar previously approved these changes Jan 9, 2025
View reviewed changes
@wpscholar wpscholar merged commit b2ce9be into main Jan 9, 2025
13 checks passed
@wpscholar wpscholar deleted the feature/PRESS12-44-GA-event-for-Woopay-connection branch January 9, 2025 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wpscholar wpscholar wpscholar approved these changes

@BrianHenryIE BrianHenryIE Awaiting requested review from BrianHenryIE

@circlecube circlecube Awaiting requested review from circlecube

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mamatharao05 @BrianHenryIE @wpscholar